-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tproxy: transparent_proxy
reference docs
#20241
Conversation
ed3a075
to
9355ec6
Compare
transparent_proxy
docstransparent_proxy
reference docs
Update the service mesh integration docs to explain how Consul needs to be configured for transparent proxy. Update the walkthrough to assume that `transparent_proxy` mode is the best approach, and move the manually-configured `upstreams` to a separate section for users who don't want to use Consul DNS. Ref: #20175 Ref: #20241
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love how you include the preview links in the PR description, very convenient!
I have a few comments and found a couple broken links. also the changeset includes a lot of actual code, which I don't think was intended for this docs PR?
aside from those things, LGTM!
api/consul.go
Outdated
TransparentProxy *ConsulTransparentProxy `mapstructure:"transparent_proxy" hcl:"transparent_proxy,block"` | ||
Config map[string]interface{} `hcl:"config,block"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious why Config made its way down here, away from its fellow uncommented fields above? is there a logic behind the field order here that I'm not seeing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's no longer in this PR, but the Config
block on all these Consul fields is a mushy "escape hatch", kind of like the task.config
block. So by moving it to the bottom I'm trying to say "this is leftover". But I see how having a comment on only the field above it makes it look like these are paired now. I'll go back and add some docstrings to the rest.
Oops, I needed to rebase on the |
Update the service mesh integration docs to explain how Consul needs to be configured for transparent proxy. Update the walkthrough to assume that `transparent_proxy` mode is the best approach, and move the manually-configured `upstreams` to a separate section for users who don't want to use Consul DNS. Ref: #20175 Ref: #20241
) Update the service mesh integration docs to explain how Consul needs to be configured for transparent proxy. Update the walkthrough to assume that `transparent_proxy` mode is the best approach, and move the manually-configured `upstreams` to a separate section for users who don't want to use Consul DNS. Ref: #20175 Ref: #20241
I've just merged the other docs PR, so I'll rebase on that and just take one more pass to make sure there are no broken links. |
) Update the service mesh integration docs to explain how Consul needs to be configured for transparent proxy. Update the walkthrough to assume that `transparent_proxy` mode is the best approach, and move the manually-configured `upstreams` to a separate section for users who don't want to use Consul DNS. Ref: #20175 Ref: #20241
) Update the service mesh integration docs to explain how Consul needs to be configured for transparent proxy. Update the walkthrough to assume that `transparent_proxy` mode is the best approach, and move the manually-configured `upstreams` to a separate section for users who don't want to use Consul DNS. Ref: #20175 Ref: #20241
This PR is targeting the feature branch, and only covers the jobspec reference docs, not the more involved integration docs that'll be under a separate PR.
Preview links:
Ref: #20175